eth: MarshalReceipt use uint for txIndex #33340
Closed
+5
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KolbyML
requested review from
fjl,
lightclient,
rjl493456442 and
s1na
as code owners
KolbyML
changed the title
Member
|
Hi, thanks a lot for your contribution! 🙏 We really appreciate the effort and encourage you to keep contributing, but for now we need to focus our limited review bandwidth on feature work, bug fixes, and protocol changes. Thanks for understanding! |
Member
Author
|
@gballet can you include the change in one of your PR's then so our upstream code is a bit more maintainable thanks 😁 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is no case where a transactionIndex can or should be negative.
TransactionIndex either uses
uintoruint64depending on the struct, so I don't think we be casting uint's to int, when callingMarshalReceipt()I think using uint64 in
MarshalReceipt()for txIndex's would be nice, butReceipt.TransactionIndexusesuint.GetCanonicalTransaction()returns a uint64.Imho
Receipt.TransactionIndexshould be using uint64, but assuming 32 bit is dead, which a lot of codebases assume, I guess it is fine. I am not aware of the full context, so I will leave that there